update each function examples
#796
Conversation
|
@josephjclark i am not sure if docs changes need a changeset, should i add one ? |
There was a problem hiding this comment.
The each explanation really isn't good, which is the bigger crime here to be honest. I'll take a look at that if you can fix the examples per my comments.
Regarding changeset - we technically don't need it. But it will result in the git tag being out of date, and it means that main has a diff from production.
So I would add a patch-level changeset on this.
packages/common/src/Adaptor.js
Outdated
| * each("$.[*]", | ||
| * create("SObject", | ||
| * field("FirstName", sourceValue("$.firstName")) | ||
| * @example <caption>Inserting patitent data using lazy state. (Only in v2)</caption> |
There was a problem hiding this comment.
Do we need to specify v1 and v2 rules here? That might create more confusion that it solves, and I don't think we have any active in-development projects on v1?
There was a problem hiding this comment.
$.data is lazy state loading and it doesn't work in v1. So i am bit worried there. I am not sure when officially we are going shutdown v1
There was a problem hiding this comment.
V1 will keep running for ages, but no-one is writing new jobs on it. Check with Aleksa.
There was a problem hiding this comment.
Paging @aleksa-krolls 🔔Can we document stuff that won't work on v1?
I think we should be focusing our docs on V2. I also think talking about v1 and V2 is confusing for users. V1 of what?
There was a problem hiding this comment.
@josephjclark I agree that we should focus the docs on v2... so no need to say Only in v2 as that should be assumed.
That said, I do think it's okay to document when an old example is from from/supported in openfn v1 ... because this helps our team and v1 users understand why old job code may look different than what they're seeing on the latest docs. But I'll leave this up to you and @mtuchi to decide.
There was a problem hiding this comment.
That's a good steer, thanks. I'll double check this and see if I want to flag anything before merging, although I suspect not in this work
packages/common/src/Adaptor.js
Outdated
| * $.data, | ||
| * insert("patient", (state) => ({ | ||
| * patient_id: state.data.case_id, | ||
| * patient_name: state.data.properties.case_name.toUpperCase(), |
There was a problem hiding this comment.
I think there are too many props here which bloat the example. Maybe I'd just set patient_id and then spread state.data
There was a problem hiding this comment.
I don't understand what this mean, can you elaborate more ?
|
Thanks @mtuchi , I'll take this over from here |
|
Noted 👍🏽 @josephjclark but just a heads up, I didn't create a changeset for this, i am not sure if we need changeset for doc changes ? |
|
I'm gonna hold off release until I've heard back from @aleksa-krolls about the v1 stuff (I'm 98% confident on that thought) I've bumped versions but because a few packages are version locked to common - salesforce being the big one - not all adaptors will see this change |
packages/fhir-ndr-et/package.json
Outdated
| @@ -1,6 +1,6 @@ | |||
| { | |||
| "name": "@openfn/language-fhir-ndr-et", | |||
| "version": "0.1.4", | |||
| "version": "0.1.5", | |||
There was a problem hiding this comment.
Wait, does this adaptor even export each? If it does, does it actually document it?
I think I need to double check the patches on this PR because I'm not sure that every adaptor is actually affected
|
@josephjclark seems there is a merge conflict in |
|
@mtuchi please don't merge this. I'm looking into a couple of things. I'll handle it! |
Summary
I have added couple of examples that shows how to use
eachfunction and build the docs.Details
The current example for each function is very confusing especially for beginners, Often times i see mistakes between the use of
$.dataand$.data[*]. And more often how to access data in the operation when using each.AI Usage
Please disclose how you've used AI in this work (it's cool, we just want to know!):
You can read more details in our Responsible AI Policy
Review Checklist
Before merging, the reviewer should check the following items:
dev only changes don't need a changeset.